Skip to content

fix(target): preserve app context across bundle and publish - #74

Open
sunnylqm wants to merge 14 commits into
masterfrom
fix/preserve-app-target-context
Open

fix(target): preserve app context across bundle and publish#74
sunnylqm wants to merge 14 commits into
masterfrom
fix/preserve-app-target-context

Conversation

@sunnylqm

@sunnylqm sunnylqm commented Aug 29, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • resolve the app an operation targets once and reuse it everywhere in that operation (resolveAppId()), instead of each step re-reading the selected app
  • carry appId from bundle into publish, and from version creation into native-package binding
  • honor an explicit --config when selecting or creating an app
  • keep update.json as the selected-app file for both brands (see below)
  • SDK: BundleOptions.appId/config, provider.getSelectedApp(platform, config)

Problem

A single publish command could operate on more than one app context:

  • bundle --appId ... --name ... used the explicit app for the Hermes base lookup, but did not forward it to publish
  • publish --appId ... created the OTA version under that app, then the internal binding step re-read the default selected app
  • createApp --config ... selected the newly created app in the default config rather than the requested file
  • SDK callers (provider.bundle()) had no way to pin the app at all

In a multi-app project this could create a version for one app and attempt to bind it against another app's native package, or compile a Hermes delta against one app's base and upload it to another.

Changes

  • resolveAppId({ appId, config, platform }) in app.ts: explicit --appId wins, otherwise the app selected for the platform in --config (default update.json). Replaces nine hand-rolled resolution blocks in bundle.ts, versions.ts, package.ts.
  • bundle: the app is resolved before any side effect (.gitignore edits, plugin probes, Metro/Hermes). A named bundle with no selected app fails immediately; a bundle-only run tolerates only a missing selection (typed AppNotSelectedError, full-bytecode fallback) and reports a malformed config instead of swallowing it. The same app id feeds the Hermes base lookup and publish.
  • publish passes the resolved appId into both update calls (binding, deps warning).
  • createApp --config selects the new app in that file.
  • Parse/mismatch messages name the file actually read (or --appId) instead of a hard-coded update.json.

Why update.json stays the default for cresc

updateJson in constants.ts said cresc.config.json but was never used: every read/write in app.ts hard-coded update.json, the cresc docs (getting-started, integration, cli) document update.json, and the client SDK does import _updateConfig from './update.json'. Switching the default would have broken every existing cresc project with no migration path, so the constant is now update.json for both brands.

Tests

  • tests/target-context.test.ts drives bundleCommands.bundle end to end with the runner/pack/publish mocked: one config read shared by the Hermes base lookup and publish, default file, explicit --appId, fail-fast on missing selection for named bundles, bundle-only fallback, malformed config, dev bundles
  • resolveAppId unit cases (default file, explicit config, explicit id, platform validation, missing/mismatched/malformed config)
  • versions.test.ts: explicit appId is kept through upload, version creation, and binding
  • constants.test.ts: updateJson is update.json for cresc

Not changed here: --appId is still not cross-checked against the app's platform on the client (pre-existing for publish --appId; the server validates).

@coderabbitai

coderabbitai Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

Next included review available in 12 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ec968269-2f42-4ae5-8e2c-9bc4d3fe29e6

📥 Commits

Reviewing files that changed from the base of the PR and between 146c0e8 and 7e89a29.

📒 Files selected for processing (11)
  • src/app.ts
  • src/bundle.ts
  • src/locales/en.ts
  • src/locales/zh.ts
  • src/package.ts
  • src/provider.ts
  • src/types.ts
  • src/utils/constants.ts
  • src/versions.ts
  • tests/constants.test.ts
  • tests/target-context.test.ts

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 6a584397-1ed0-4962-90e5-f3f4d05c13d9

📥 Commits

Reviewing files that changed from the base of the PR and between 49fa3e8 and 146c0e8.

📒 Files selected for processing (2)
  • src/app.ts
  • src/bundle.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/bundle.ts
  • src/app.ts

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Bundle publishing now carries explicit appId and config values through normalization, request construction, app selection, and version updates. App creation reads and writes the selected configuration path. Tests cover target resolution, configuration targeting, bundle publishing, and version binding.

Changes

Target context propagation

Layer / File(s) Summary
Application configuration targeting
src/app.ts, tests/target-context.test.ts
Application selection and creation accept an explicit configuration path. Target resolution supports explicit appId values, memoizes successful lookups, and retries after failures.
Bundle target propagation
src/bundle.ts, tests/target-context.test.ts
Bundle normalization preserves appId and config. Publish request construction, Hermes lookup, and publishing paths use the resolved target.
Version app binding
src/versions.ts, tests/versions.test.ts
Selector-driven and interactive package updates now receive the resolved appId.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 146c0

This change preserves the selected app and configuration through bundling, publishing, version creation, and native-package binding. No actionable merge-blocking risk remains beyond normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant CLI
  participant normalizeBundleOptions
  participant createAppTargetResolver
  participant createPublishBundleRequest
  participant PublishAPI
  participant versionCommands
  CLI->>normalizeBundleOptions: provide appId and config
  normalizeBundleOptions->>createAppTargetResolver: resolve target context
  createAppTargetResolver-->>createPublishBundleRequest: return appId and configPath
  createPublishBundleRequest->>PublishAPI: submit bundle publish request
  PublishAPI-->>CLI: return version ID
  CLI->>versionCommands: update version with appId
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 5 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: preserving app context across bundle and publish workflows.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/preserve-app-target-context

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

sunnylqm commented Aug 29, 2026

Copy link
Copy Markdown
Collaborator Author

全部评审意见已逐项落实:

  • 新增 operation-scoped app target resolver,同一次 bundle 操作只解析一次 selected-app 配置。
  • Hermes base、named publish、interactive publish 和原生包 binding 始终复用同一个 appId,避免构建期间配置切换造成跨应用发布。
  • bundle --name 在耗时构建前完成目标应用预检;纯 bundle 的自动 Hermes base 查询仍保持 best-effort,并支持失败后重试。
  • createApp --configselectApp --config 和 Cresc 的 cresc.config.json 默认路径保持一致。
  • 已补齐所有受影响目标解析、应用命令和 bundle/publish helper 的 JSDoc,覆盖 CodeRabbit 提出的 docstring coverage 警告;按其上一轮统计的 12 个受影响函数计算,除原有 versionCommands.publish 外均已有对应 JSDoc,覆盖率高于 80% 门槛。
  • 回归测试覆盖配置从 app 42 切换到 app 99、失败后重试、显式 appId 不读取配置、上传/创建版本/绑定使用同一应用,以及自定义配置文件写入。

最新提交:146c0e8

GitHub Actions run #175 全部通过:

  • lint / Biome formatting ✅
  • TypeScript typecheck ✅
  • 完整单元测试与覆盖率 ✅
  • npm publish dry-run ✅

CodeRabbit 对功能变更的最近一次完整增量评审结论为 No actionable commentsMerge Risk: Minimal;当前无未解决 review thread,最新 commit status 亦为 success。

@coderabbitai

coderabbitai Bot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

@sunnylqm 已收到。将对当前 PR 提交执行增量评审。

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@sunnylqm

Copy link
Copy Markdown
Collaborator Author

@coderabbitai

@coderabbitai

coderabbitai Bot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

@sunnylqm 已收到。请说明需要的操作,例如重新评审、批准或检查特定问题。

You are interacting with an AI system.

sunnylqm added a commit that referenced this pull request Aug 30, 2026
…for both brands

Follow-up to the review of #74.

- keep `update.json` as the selected-app file for cresc too: the cresc docs
  and the client SDK read that name, and `cresc.config.json` was never wired
  in, so switching the default would have broken every existing cresc project
- replace the nine hand-rolled `options.appId || getSelectedApp(...)` blocks
  in bundle/versions/package with one `resolveAppId()` helper
- bundle: resolve the app before any side effect (.gitignore edits, plugin
  probes) so a named bundle without a selected app fails immediately; a
  bundle-only run only tolerates a missing selection (typed
  AppNotSelectedError) and reports malformed configs instead of swallowing
  them; drop the dead `config` forwarding and the three-way cached target
- SDK: `BundleOptions.appId/config` and `provider.getSelectedApp(platform,
  config)` so programmatic callers get the same single-app guarantee
- messages: parse/mismatch errors name the file (or `--appId`) actually used
- tests: exercise bundleCommands.bundle end to end (Hermes base + publish get
  the same app, fail-fast, bundle-only fallback, dev bundles) and the default
  file, instead of the removed wrapper helpers

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JCaS35vZG4DCtmM24MYaVR
sunnylqm added a commit that referenced this pull request Aug 30, 2026
…for both brands

Follow-up to the review of #74.

- keep `update.json` as the selected-app file for cresc too: the cresc docs
  and the client SDK read that name, and `cresc.config.json` was never wired
  in, so switching the default would have broken every existing cresc project
- replace the nine hand-rolled `options.appId || getSelectedApp(...)` blocks
  in bundle/versions/package with one `resolveAppId()` helper
- bundle: resolve the app before any side effect (.gitignore edits, plugin
  probes) so a named bundle without a selected app fails immediately; a
  bundle-only run only tolerates a missing selection (typed
  AppNotSelectedError) and reports malformed configs instead of swallowing
  them; drop the dead `config` forwarding and the three-way cached target
- SDK: `BundleOptions.appId/config` and `provider.getSelectedApp(platform,
  config)` so programmatic callers get the same single-app guarantee
- messages: parse/mismatch errors name the file (or `--appId`) actually used
- tests: exercise bundleCommands.bundle end to end (Hermes base + publish get
  the same app, fail-fast, bundle-only fallback, dev bundles) and the default
  file, instead of the removed wrapper helpers

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JCaS35vZG4DCtmM24MYaVR
@sunnylqm
sunnylqm force-pushed the fix/preserve-app-target-context branch from de0cdc6 to ac3ede8 Compare August 30, 2026 02:40
…for both brands

Follow-up to the review of #74.

- keep `update.json` as the selected-app file for cresc too: the cresc docs
  and the client SDK read that name, and `cresc.config.json` was never wired
  in, so switching the default would have broken every existing cresc project
- replace the nine hand-rolled `options.appId || getSelectedApp(...)` blocks
  in bundle/versions/package with one `resolveAppId()` helper
- bundle: resolve the app before any side effect (.gitignore edits, plugin
  probes) so a named bundle without a selected app fails immediately; a
  bundle-only run only tolerates a missing selection (typed
  AppNotSelectedError) and reports malformed configs instead of swallowing
  them; drop the dead `config` forwarding and the three-way cached target
- SDK: `BundleOptions.appId/config` and `provider.getSelectedApp(platform,
  config)` so programmatic callers get the same single-app guarantee
- messages: parse/mismatch errors name the file (or `--appId`) actually used
- tests: exercise bundleCommands.bundle end to end (Hermes base + publish get
  the same app, fail-fast, bundle-only fallback, dev bundles) and the default
  file, instead of the removed wrapper helpers

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JCaS35vZG4DCtmM24MYaVR
@sunnylqm
sunnylqm force-pushed the fix/preserve-app-target-context branch from ac3ede8 to 7e89a29 Compare August 30, 2026 02:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant